OMNI: Fix dimension range-indexing in frontend #363
Conversation
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/363/index.html |
mlange05
force-pushed
the
naml-omni-fix-range-indexing
branch
5 times, most recently
from
ff1a188
to
5a10e51
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #363 +/- ##
==========================================
- Coverage 95.41% 95.40% -0.02%
==========================================
Files 177 178 +1
Lines 37332 37267 -65
==========================================
- Hits 35622 35555 -67
- Misses 1710 1712 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This utility was really just used to revert OMNI funckiness...
mlange05
force-pushed
the
naml-omni-fix-range-indexing
branch
from
5a10e51
to
7fed99e
Compare
loki/frontend/util.py
Outdated
| for v in self.retriever.retrieve(o.symbols): | ||
| dimensions = tuple(d.upper if self.is_one_index(d) else d for d in v.dimensions) | ||
| _type = v.type | ||
| if v.shape: |
There was a problem hiding this comment.
Note that v.shape is only an indirection to v.type.shape:
loki/loki/expression/symbols.py
Lines 746 to 750 in 9a76c21
Since you've already looked up the type, you might as well use _type.shape here and in the line below, to avoid hitting the symbol table again
There was a problem hiding this comment.
Good spot! Pushed a fix.
reuterbal
added
the
ready for merge
Just testing for now ...This change adds a frontend sanitisation step to remove the forced lower bounds on array declaration dimensions in OMNI. This is done via a custom sanitisation
Transformer.As a result, we can remove a lot of the special-casing for OMNI's array dimension notation and, importantly, remove the use of the
normalize_range_indexingutility across theloki.transformationsub-package tests. Consequently, we can also remove the OMNI special-casing inloki_transform.py- the problem that triggered this entire endeavour.Please note, the
normalize_range_indexingtool has not been removed, as it has a legitimate use in another sanitisation utility, which is part of the C-transpilation toolchain. However, all OMNI-specific workarounds using this utility have been removed.